otelhttp: handle request Pattern#7192
Conversation
|
This has the downside of running the span name formatter twice if there's a pattern. |
|
cc @seankhliao @axw |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #7192 +/- ##
=======================================
- Coverage 81.2% 81.2% -0.1%
=======================================
Files 207 207
Lines 18271 18275 +4
=======================================
+ Hits 14844 14845 +1
- Misses 3006 3008 +2
- Partials 421 422 +1
🚀 New features to boost your workflow:
|
axw
left a comment
axw
left a comment
There was a problem hiding this comment.
Neat.
One other downside of changing the name after handling the request is that it won't work well with samplers that care about the span name, like the per-operation Jaeger remote sampler, IIANM.
|
I can add a comment to the WithSpanNameFormatter method for better awareness of that. |
seankhliao
left a comment
seankhliao
left a comment
There was a problem hiding this comment.
It's a cool trick,
it may be a bit fragile if someone uses some other middleware that clones the request (uses WithContext) between otelhttp and the mux, only the new copy will have the Pattern set.
Can we have a spanformatter func or option to use the pattern (a separate pr?)
I'd make that a separate PR, yes. Do you want to open one to better articulate what you need? |
|
sure I can make one |
|
I was thinking something simple like a44c60f (currently based on top of this pr). |
Co-authored-by: Robert Pająk <pellared@hotmail.com>
Co-authored-by: Robert Pająk <pellared@hotmail.com>
|
@XSAM PTAL |
#### Description Update the confighttp server span names to use the low-cardnality request pattern, rather than the full client-specified path, as described by the specification: https://opentelemetry.io/docs/specs/semconv/http/http-spans/#name. Requires open-telemetry/opentelemetry-go-contrib#7192 #### Link to tracking issue Fixes #12468 #### Testing Added a unit test. #### Documentation None --------- Co-authored-by: Alex Boten <223565+codeboten@users.noreply.github.com>
7 participants
Closes #6193
Handles otelhttp for #6919 (@Ananyasinha13 I know you're assigned this, but it has been 2 weeks with no movement).